Phase 3: honest EntitlementService (LSP fix)#9
Merged
Conversation
added 4 commits
July 19, 2026 01:18
- Rewrite AGENTS.md/README design system to the live 'Field Manual' light theme (the 'Ethereal Glass' dark design was reverted) - Correct tech stack: PostgreSQL-only (no SQLite), Bun/CI-npm, pdfkit note - Fix REMEDIATION_PLAN false claims (download route is 53 lines, not 879; SQLite) - Mark REDESIGN-PLAN.md as superseded - Delete truly-dead code (zero references): browser-llm-integration.ts (338-line unused fake rule-based AI) - NOTE: UpgradeModal/SubscriptionBanner/pricing.ts are compile-time shims still imported by components; their removal is deferred to Phase 3/4 (entitlement + component refactor), not Phase 0. - Add FIX-PLAN.md tracking the full SOLID/UI remediation roadmap
Extract shared AI logic out of the 4 route handlers into a single tested
abstraction, fixing SRP/OCP/DIP violations:
- src/lib/ai/json.ts: central JSON extraction from free-text LLM responses
- src/lib/ai/client.ts: AIProvider interface + ZAIProvider with 30s timeout
and abort support (swap provider without touching routes — DIP)
- src/lib/ai/handlers.ts: createAIHandler factory centralizes auth, input
validation, model call, parsing, and error handling (OCP: add a feature
by adding a config, not copying a route)
- src/lib/ai/{coach,resume,cover-letter,assessment}.ts: per-feature prompt
+ validation + fallback configs (SRP)
Routes shrink to single-line exports. Behavior is preserved exactly:
coach/cover-letter return graceful fallbacks on parse failure; resume/
assessment return 500. No new dependencies (validateShape replaces zod).
Tests: add 17 unit tests for json.ts + handlers.ts (all passing).
tsc --noEmit and eslint clean.
The export route was a 200-line monolith that owned auth, DOCX generation, and a hand-rolled PDF builder that SILENTLY TRUNCATED content at the bottom of the page (if (y < 50) break). Changes: - src/lib/export/types.ts: shared ExportRequest + MAX_CONTENT_LENGTH (50k) + filename sanitizer - src/lib/export/docx.ts: DOCX builder extracted (SRP) - src/lib/export/pdf.ts: PDF builder extracted AND paginated. Long documents now flow onto new pages instead of being cut off — fixes the data-loss bug. - src/app/api/export/route.ts: shrunk to ~50 lines — auth, validation, type dispatch, and a content-length guard only. Kept the manual Courier PDF approach (no external fonts) to avoid the pdfkit font-resolution issues noted in the original code, rather than adding a new dependency. Tests: add 11 unit tests (pdf pagination/no-truncation, docx validity, helpers). tsc --noEmit and eslint clean.
The subscription guard returned allowed:true for everything but its function signatures IMPLIED tier gating (LSP violation) — a caller could not tell whether gating was real or a silently-passing no-op. use-subscription returned magic -1 sentinels for 'unlimited'. Changes: - src/lib/subscription/entitlement.ts: explicit EntitlementService interface + FreeEntitlement impl (tier 'free', all features allowed, limits = null = unlimited). Swap the exported singleton to add paid tiers later with zero caller changes (DIP). - src/lib/subscription-guard.ts: check*Access() helpers now delegate to the EntitlementService. Signatures unchanged so existing callers keep working. - src/lib/use-subscription.ts: returns honest null (unlimited) limits sourced from the service instead of magic -1. - Deleted src/lib/pricing.ts (tier config that only powered the stubs). Tests: add 5 unit tests for the entitlement service. tsc --noEmit and eslint clean.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (15)
📒 Files selected for processing (18)
📝 WalkthroughWalkthroughThe change updates platform documentation, centralizes AI route handling, extracts DOCX/PDF export utilities, introduces entitlement-backed access, removes pricing and obsolete browser AI code, updates application assets and metadata, and adds focused unit tests plus recorded test outputs. ChangesPlatform documentation and remediation
AI request pipeline
Document export pipeline
Entitlement-backed access
Application alignment
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant AIRoute as AI route
participant Handler as createAIHandler
participant Auth as getUserFromRequest
participant AIClient as completeJson
Client->>AIRoute: Submit AI request
AIRoute->>Handler: Invoke configured handler
Handler->>Auth: Check authentication
Handler->>AIClient: Send system and user prompts
AIClient-->>Handler: Return parsed JSON or null
Handler-->>Client: Return result or HTTP error
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
REMEDIATION_PLAN.md (1)
115-120: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove completed items from the pending fix list.
This section says the README is already corrected and standalone output already exists, but the Fix list still asks contributors to update the README and add
output: "standalone". Mark those tasks complete or remove them to prevent redundant work.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@REMEDIATION_PLAN.md` around lines 115 - 120, Update the pending Fix list in the remediation plan to remove or mark complete the README PostgreSQL/runtime documentation task and the next.config.ts standalone output task, since both are already implemented. Keep only genuinely outstanding remediation items.
🧹 Nitpick comments (5)
src/lib/subscription/entitlement.ts (1)
16-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the tier contract extensible.
Tier = 'free'prevents a futurePaidEntitlementwithtier: 'paid'from implementingEntitlementService, despite the documented swap-in design. Include intended paid identifiers (for example,'free' | 'paid') or use an intentionally extensible tier identifier.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/subscription/entitlement.ts` at line 16, Update the Tier type alias in entitlement.ts to accept both current and intended entitlement tiers, including 'free' and 'paid', or use an intentionally extensible tier identifier so future PaidEntitlement implementations can satisfy EntitlementService.__tests__/unit/entitlement.test.ts (1)
1-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMirror the source path in the test layout.
Move this to
__tests__/lib/subscription/entitlement.test.tsso it mirrorssrc/lib/subscription/entitlement.ts.As per coding guidelines, “Place tests under
__tests__/, mirroring thesrc/structure.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@__tests__/unit/entitlement.test.ts` around lines 1 - 6, Move the entitlement test file from the current unit-test location into the mirrored __tests__/lib/subscription structure, preserving its existing imports and test content so it aligns with src/lib/subscription/entitlement.ts.Source: Coding guidelines
__tests__/unit/export-docx.test.ts (1)
1-2: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove export tests into the mirrored
lib/exporttest path.
__tests__/unit/export-docx.test.ts#L1-L2: move to__tests__/lib/export/docx.test.ts.__tests__/unit/export-pdf.test.ts#L1-L3: move to__tests__/lib/export/pdf.test.ts.As per coding guidelines, tests under
__tests__/must mirror thesrc/structure.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@__tests__/unit/export-docx.test.ts` around lines 1 - 2, Move the generateDocx tests from __tests__/unit/export-docx.test.ts to __tests__/lib/export/docx.test.ts, and move the generatePdf tests from __tests__/unit/export-pdf.test.ts to __tests__/lib/export/pdf.test.ts. Preserve the existing test contents and imports while aligning both files with the mirrored src/lib/export structure.Source: Coding guidelines
src/lib/ai/client.ts (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the
@/alias for this source import.As per coding guidelines, “Use the
@/path alias for imports that map tosrc/.”Proposed change
-import { extractJson } from './json'; +import { extractJson } from '`@/lib/ai/json`';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/ai/client.ts` at line 1, Update the extractJson import in client.ts to use the "`@/`..." path alias corresponding to its src location, replacing the relative './json' import without changing behavior.Source: Coding guidelines
__tests__/unit/ai-handlers.test.ts (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove this suite to mirror the source path.
Place it at
__tests__/lib/ai/handlers.test.tsso it mirrorssrc/lib/ai/handlers.ts.As per coding guidelines, “Place tests under
__tests__/, mirroring thesrc/structure.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@__tests__/unit/ai-handlers.test.ts` at line 1, Move the test suite from its current location to __tests__/lib/ai/handlers.test.ts so it mirrors the source module src/lib/ai/handlers.ts. Preserve the existing test contents and behavior unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@__tests__/unit/ai-json.test.ts`:
- Around line 1-2: Move the test containing extractJson coverage from its
current location to __tests__/lib/ai/json.test.ts so the test directory mirrors
the source structure for src/lib/ai/json.ts; preserve the existing describe, it,
and expect test contents unchanged.
In `@FIX-PLAN.md`:
- Line 3: Replace the future-dated July 19, 2026 audit entry in FIX-PLAN.md line
3 with the actual audit date, and update the July 19, 2026 README correction
date in REMEDIATION_PLAN.md line 115 to the actual change date.
- Line 58: Update the verification steps in FIX-PLAN.md to reference the current
test files under __tests__/, replacing outdated __tests__/ai/client.test.ts,
handlers.test.ts, and subscription.test.ts references with the appropriate
__tests__/unit/ai-handlers.test.ts, __tests__/unit/ai-json.test.ts, and
__tests__/unit/entitlement.test.ts paths. Keep the existing bun run test
command.
- Around line 30-85: Update FIX-PLAN.md to reflect the implemented state: mark
the shared AI pipeline, extracted export modules, and entitlement service work
in Phases 1–3 as complete, including their related verification items where
already satisfied. Remove or rewrite instructions that would reimplement
existing functionality, and leave only genuinely outstanding gaps from the
current implementation.
In `@README.md`:
- Around line 103-113: Update the README project structure to remove the stale
pricing.ts entry, replace the outdated subscription-guard description, and add
the supported src/lib/subscription/entitlement.ts entitlement service with an
accurate access-control description.
- Line 40: Clarify the package-manager guidance in the README’s Runtime entry so
it explicitly states that Bun is used for local development while npm is used in
CI, and ensure it does not conflict with the AGENTS.md contributor instructions.
- Around line 152-153: Update the README test documentation to consistently
reflect the current 187-test total, replacing the outdated 218-test API suite
count and removing the obsolete subscription.test.ts entry. Ensure the table and
related test references also include the new entitlement test coverage.
In `@src/app/api/export/route.ts`:
- Around line 14-27: Validate the parsed fields in the export route before using
them: require type to be a string, content to be a string, and title to be a
string when provided. Keep the existing required-field, export-type, and
content-length checks, and return the existing 400 validation response instead
of allowing invalid arrays or objects to reach content.split() or
sanitizeFilename().
In `@src/lib/ai/client.ts`:
- Around line 20-79: Update ZAIProvider.complete and withTimeout so the single
timeout covers SDK import, ZAI.create(), and completion creation, while
preserving cleanup. Make opts.signal abort actively reject the returned promise
(and settle only once), since the SDK request cannot consume the signal; ensure
both external cancellation and timeout abort the controller and produce the
appropriate rejection.
In `@src/lib/ai/coach.ts`:
- Around line 98-107: Update the coaching input validation in validate and
buildUserPrompt to enforce per-field limits for question, userAnswer, and
questionContext, plus a total prompt-size limit before provider submission.
Reject or safely handle oversized inputs with the existing validation error
shape, and ensure only bounded values are interpolated into the prompt.
- Around line 19-34: Add a truthWarnings: string[] field to the coaching
prompt/schema and populate it in both fallback responses. Ensure generated
coaching results expose warnings for unsupported or exaggerated claims while
preserving the existing rubricBreakdown.truthfulness score.
In `@src/lib/ai/handlers.ts`:
- Around line 9-20: Update validateShape to validate each required field’s
expected type and non-empty value before returning ok: true, rather than only
checking for undefined properties. Ensure invalid values such as null
jobDescription or numeric question are reported in missing/invalid fields so
callers cannot cast malformed request bodies before invoking the model.
- Around line 74-90: Update AIHandlerConfig and the handler around completeJson
to validate parsed model output against each endpoint’s response contract before
normalization and response generation. Add a result-validator/fallback
configuration, invoke it after completeJson returns, and route invalid results
through the configured failure behavior instead of returning HTTP 200; preserve
the existing parse-failure handling for syntactically invalid or absent
responses.
In `@src/lib/ai/json.ts`:
- Around line 16-34: Replace the greedy object-first and array regex matching in
the JSON parsing helper with a quote-aware balanced-bracket scan that extracts
the earliest complete JSON value, regardless of whether it is an object or
array. Ensure prose-wrapped arrays containing objects and consecutive JSON
values return the first valid value, and add regression coverage for both cases.
In `@src/lib/export/pdf.ts`:
- Around line 35-36: Use one consistent WinAnsi-compatible byte encoding
throughout the PDF stream generation around allLines: encode every emitted text
value, including the bullet character and other Unicode content, before
calculating `/Length` and reuse those exact bytes when writing the stream. Do
not calculate lengths from UTF-8 strings while emitting Latin-1 bytes;
alternatively, switch the embedded font and stream handling to a Unicode-capable
encoding.
- Around line 23-39: Update the line-processing flow in the PDF export routine
to wrap each rendered text line to the available page width before pagination
and PDF text operations are generated. Preserve the existing heading, bullet,
indentation, font-size, and color metadata for every wrapped segment, and apply
the same behavior to the related pagination/rendering paths referenced by the
comment.
- Around line 82-111: Correct the object allocation and references in the PDF
generation flow around addObject, ensuring the catalog points to the actual
Pages object and each page’s /Contents, /Parent, and font resources reference
the object IDs returned for those objects rather than hard-coded positions.
Preserve a consistent ordering for catalog, pages, content streams, page
objects, and fonts, and add a parser/open round-trip test that verifies the
generated PDF can be successfully opened.
In `@src/lib/subscription-guard.ts`:
- Around line 16-27: Update guard and each feature wrapper to construct Usage
from the wrapper’s supplied count instead of the shared zero-valued usage
object. Preserve entitlement.canAccess(feature) as the access gate, and when
access is granted return the full result from entitlement.checkUsage(feature,
usage), including remaining and reason; retain the existing denied result when
feature access fails.
---
Outside diff comments:
In `@REMEDIATION_PLAN.md`:
- Around line 115-120: Update the pending Fix list in the remediation plan to
remove or mark complete the README PostgreSQL/runtime documentation task and the
next.config.ts standalone output task, since both are already implemented. Keep
only genuinely outstanding remediation items.
---
Nitpick comments:
In `@__tests__/unit/ai-handlers.test.ts`:
- Line 1: Move the test suite from its current location to
__tests__/lib/ai/handlers.test.ts so it mirrors the source module
src/lib/ai/handlers.ts. Preserve the existing test contents and behavior
unchanged.
In `@__tests__/unit/entitlement.test.ts`:
- Around line 1-6: Move the entitlement test file from the current unit-test
location into the mirrored __tests__/lib/subscription structure, preserving its
existing imports and test content so it aligns with
src/lib/subscription/entitlement.ts.
In `@__tests__/unit/export-docx.test.ts`:
- Around line 1-2: Move the generateDocx tests from
__tests__/unit/export-docx.test.ts to __tests__/lib/export/docx.test.ts, and
move the generatePdf tests from __tests__/unit/export-pdf.test.ts to
__tests__/lib/export/pdf.test.ts. Preserve the existing test contents and
imports while aligning both files with the mirrored src/lib/export structure.
In `@src/lib/ai/client.ts`:
- Line 1: Update the extractJson import in client.ts to use the "`@/`..." path
alias corresponding to its src location, replacing the relative './json' import
without changing behavior.
In `@src/lib/subscription/entitlement.ts`:
- Line 16: Update the Tier type alias in entitlement.ts to accept both current
and intended entitlement tiers, including 'free' and 'paid', or use an
intentionally extensible tier identifier so future PaidEntitlement
implementations can satisfy EntitlementService.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 085cb93b-6a7c-4c3b-b10b-7a7454d69efd
📒 Files selected for processing (30)
AGENTS.mdFIX-PLAN.mdREADME.mdREDESIGN-PLAN.mdREMEDIATION_PLAN.md__tests__/unit/ai-handlers.test.ts__tests__/unit/ai-json.test.ts__tests__/unit/entitlement.test.ts__tests__/unit/export-docx.test.ts__tests__/unit/export-pdf.test.tssrc/app/api/ai/assessment-score/route.tssrc/app/api/ai/coach/route.tssrc/app/api/ai/cover-letter/route.tssrc/app/api/ai/resume-review/route.tssrc/app/api/export/route.tssrc/lib/ai/assessment.tssrc/lib/ai/client.tssrc/lib/ai/coach.tssrc/lib/ai/cover-letter.tssrc/lib/ai/handlers.tssrc/lib/ai/json.tssrc/lib/ai/resume.tssrc/lib/browser-llm-integration.tssrc/lib/export/docx.tssrc/lib/export/pdf.tssrc/lib/export/types.tssrc/lib/pricing.tssrc/lib/subscription-guard.tssrc/lib/subscription/entitlement.tssrc/lib/use-subscription.ts
💤 Files with no reviewable changes (2)
- src/lib/pricing.ts
- src/lib/browser-llm-integration.ts
Comment on lines
+1
to
+2
| import { describe, it, expect } from 'vitest'; | ||
| import { extractJson } from '@/lib/ai/json'; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Mirror the source path under __tests__/.
Move this test to __tests__/lib/ai/json.test.ts so its location mirrors src/lib/ai/json.ts.
As per coding guidelines, “Place tests under __tests__/, mirroring the src/ structure.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@__tests__/unit/ai-json.test.ts` around lines 1 - 2, Move the test containing
extractJson coverage from its current location to __tests__/lib/ai/json.test.ts
so the test directory mirrors the source structure for src/lib/ai/json.ts;
preserve the existing describe, it, and expect test contents unchanged.
Source: Coding guidelines
| @@ -0,0 +1,133 @@ | |||
| # Interview Lab — Thorough Fix Plan | |||
|
|
|||
| > Audited 2026-07-19. A sequenced, merge-ready plan. Each phase: **goal → files → changes → verification**. | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove future-dated documentation entries.
Both files contain dates after the current date, July 18, 2026. Replace them with the actual audit/change dates.
FIX-PLAN.md#L3-L3: replace the July 19, 2026 audit date.REMEDIATION_PLAN.md#L115-L115: replace the July 19, 2026 README correction date.
📍 Affects 2 files
FIX-PLAN.md#L3-L3(this comment)REMEDIATION_PLAN.md#L115-L115
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@FIX-PLAN.md` at line 3, Replace the future-dated July 19, 2026 audit entry in
FIX-PLAN.md line 3 with the actual audit date, and update the July 19, 2026
README correction date in REMEDIATION_PLAN.md line 115 to the actual change
date.
Comment on lines
+30
to
+85
| ## Phase 1 — SOLID: AI Layer Refactor (SRP + OCP + DIP) | ||
| *Highest leverage: 4 routes share ~75-line duplicated JSON-extract loop, each calls the SDK directly.* | ||
|
|
||
| ### 1.1 `src/lib/ai/client.ts` — AIProvider abstraction (DIP) | ||
| ```ts | ||
| export interface AIProvider { | ||
| complete(system: string, user: string, opts?: { schema?: ZodSchema; signal?: AbortSignal }): Promise<unknown>; | ||
| } | ||
| export class ZAIProvider implements AIProvider { /* timeout 30s, abort, retry-once */ } | ||
| export const ai: AIProvider = new ZAIProvider(); | ||
| ``` | ||
| - Add `AbortSignal.timeout(30000)` + `try/catch` around `ZAI.create().chat.completions.create`. | ||
|
|
||
| ### 1.2 `src/lib/ai/handlers.ts` — `createAIHandler` factory (OCP) | ||
| ```ts | ||
| export function createAIHandler<T>(buildPrompt, schema: ZodSchema<T>) { | ||
| return async (req) => { /* auth → call ai.complete with schema → validate → return */ }; | ||
| } | ||
| ``` | ||
| - Centralize `extractJsonArray`/`extractJsonObject` parsing into tested util `src/lib/ai/json.ts`. | ||
|
|
||
| ### 1.3 Split routes | ||
| - `src/lib/ai/coach.ts`, `resume.ts`, `cover-letter.ts`, `assessment.ts` — each exports `buildPrompt()` + a zod `schema`. | ||
| - Refactor `src/app/api/ai/{coach,resume-review,cover-letter,assessment-score}/route.ts` to ~15 lines each calling `createAIHandler`. | ||
|
|
||
| ### 1.4 Add zod schemas | ||
| - `src/lib/ai/schemas.ts`: `CoachFeedbackSchema`, `ResumeReviewSchema`, `CoverLetterSchema`, `AssessmentScoreSchema` (replaces runtime `as` casts). | ||
|
|
||
| **Verify:** `__tests__/ai/client.test.ts` (mock `z-ai-web-dev-sdk`), `handlers.test.ts` (valid + malformed JSON → 500), `bun run test`. | ||
|
|
||
| --- | ||
|
|
||
| ## Phase 2 — SOLID: Export Layer (SRP) | ||
| ### 2.1 `src/lib/export/docx.ts` | ||
| - Move `generateDocx` out of `export/route.ts` into its own module. | ||
|
|
||
| ### 2.2 `src/lib/export/pdf.ts` — replace hand-rolled byte builder | ||
| - Use already-installed `pdfkit` (currently unused). Fixes **silent truncation** (`if (y < 50) break`) by paginating. | ||
| - Add `content` size guard (reject > 50k chars) to prevent abuse. | ||
|
|
||
| ### 2.3 `src/app/api/export/route.ts` | ||
| - Becomes ~12 lines: auth → dispatch to `docx.ts` / `pdf.ts`. | ||
|
|
||
| **Verify:** `export.test.ts` generates real .docx/.pdf, asserts multi-page content isn't truncated. | ||
|
|
||
| --- | ||
|
|
||
| ## Phase 3 — SOLID: Entitlement Honesty (LSP) | ||
| ### 3.1 `src/lib/subscription/entitlement.ts` | ||
| ```ts | ||
| export interface EntitlementService { canAccess(feature): boolean; tier: string; } | ||
| export class FreeEntitlement implements EntitlementService { canAccess() { return true; } } | ||
| ``` | ||
| - `subscription-guard.ts` returns real interface; `use-subscription.ts` returns `FreeEntitlement` honestly (remove fake `-1`/no-op that *implies* gating). | ||
|
|
||
| **Verify:** `subscription.test.ts`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Reconcile the plan with the implementation state.
The current PR already introduces the shared AI pipeline, extracted export modules, and entitlement service, but Phases 1–3 still present these as upcoming work. Mark landed items complete and leave only genuinely outstanding gaps to avoid reimplementing shipped functionality.
Based on the PR objectives and stack context, these service layers are already implemented.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@FIX-PLAN.md` around lines 30 - 85, Update FIX-PLAN.md to reflect the
implemented state: mark the shared AI pipeline, extracted export modules, and
entitlement service work in Phases 1–3 as complete, including their related
verification items where already satisfied. Remove or rewrite instructions that
would reimplement existing functionality, and leave only genuinely outstanding
gaps from the current implementation.
| ### 1.4 Add zod schemas | ||
| - `src/lib/ai/schemas.ts`: `CoachFeedbackSchema`, `ResumeReviewSchema`, `CoverLetterSchema`, `AssessmentScoreSchema` (replaces runtime `as` casts). | ||
|
|
||
| **Verify:** `__tests__/ai/client.test.ts` (mock `z-ai-web-dev-sdk`), `handlers.test.ts` (valid + malformed JSON → 500), `bun run test`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Point verification steps at the current test layout.
The plan references __tests__/ai/client.test.ts and subscription.test.ts, while the current repository uses paths such as __tests__/unit/ai-handlers.test.ts, __tests__/unit/ai-json.test.ts, and __tests__/unit/entitlement.test.ts. Update these commands to match the actual test structure.
As per coding guidelines, tests belong under __tests__/ while mirroring the source structure.
Also applies to: 85-85
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@FIX-PLAN.md` at line 58, Update the verification steps in FIX-PLAN.md to
reference the current test files under __tests__/, replacing outdated
__tests__/ai/client.test.ts, handlers.test.ts, and subscription.test.ts
references with the appropriate __tests__/unit/ai-handlers.test.ts,
__tests__/unit/ai-json.test.ts, and __tests__/unit/entitlement.test.ts paths.
Keep the existing bun run test command.
Source: Coding guidelines
| | **Framework** | Next.js 16.1.1 (App Router, standalone) | | ||
| | **Runtime** | Bun | | ||
| | **Database** | Prisma v6 + SQLite (dev) / PostgreSQL (prod) | | ||
| | **Runtime** | Bun (CI uses npm) | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Clarify the package-manager scope.
README.md says CI uses npm, while AGENTS.md Line 71 says “Use Bun exclusively.” Clarify that Bun is for local development and npm is CI, or update AGENTS.md to avoid conflicting contributor instructions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` at line 40, Clarify the package-manager guidance in the README’s
Runtime entry so it explicitly states that Bun is used for local development
while npm is used in CI, and ensure it does not conflict with the AGENTS.md
contributor instructions.
Comment on lines
+16
to
+34
| const objectMatch = trimmed.match(/\{[\s\S]*\}/); | ||
| if (objectMatch) { | ||
| try { | ||
| return JSON.parse(objectMatch[0]) as T; | ||
| } catch { | ||
| // malformed; fall through | ||
| } | ||
| } | ||
|
|
||
| const arrayMatch = trimmed.match(/\[[\s\S]*\]/); | ||
| if (arrayMatch) { | ||
| try { | ||
| return JSON.parse(arrayMatch[0]) as T; | ||
| } catch { | ||
| // malformed | ||
| } | ||
| } | ||
|
|
||
| return null; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Extract the earliest complete JSON value instead of using greedy regexes.
The object-first matching returns the wrong shape for prose-wrapped arrays such as [{"a":1}], while multiple objects are greedily merged and rejected. Use a quote-aware balanced-bracket scan and add regression cases for arrays containing objects and consecutive JSON values.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/ai/json.ts` around lines 16 - 34, Replace the greedy object-first and
array regex matching in the JSON parsing helper with a quote-aware
balanced-bracket scan that extracts the earliest complete JSON value, regardless
of whether it is an object or array. Ensure prose-wrapped arrays containing
objects and consecutive JSON values return the first valid value, and add
regression coverage for both cases.
Comment on lines
+23
to
+39
| for (const line of lines) { | ||
| const trimmed = line.trim(); | ||
| if (!trimmed) { | ||
| allLines.push({ text: '', size: 4, bold: false, indent: 0, color: '0 0 0' }); | ||
| continue; | ||
| } | ||
| if (trimmed.startsWith('### ')) { | ||
| allLines.push({ text: trimmed.replace('### ', ''), size: 12, bold: true, indent: 0, color: '0 0 0' }); | ||
| } else if (trimmed.startsWith('## ')) { | ||
| allLines.push({ text: trimmed.replace('## ', ''), size: 14, bold: true, indent: 0, color: '0 0 0' }); | ||
| } else if (trimmed.startsWith('# ')) { | ||
| allLines.push({ text: trimmed.replace('# ', ''), size: 16, bold: true, indent: 0, color: '0 0 0' }); | ||
| } else if (trimmed.startsWith('- ') || trimmed.startsWith('• ')) { | ||
| allLines.push({ text: `• ${trimmed.replace(/^[-•]\s*/, '')}`, size: 11, bold: false, indent: 20, color: '0 0 0' }); | ||
| } else { | ||
| allLines.push({ text: trimmed, size: 11, bold: false, indent: 0, color: '0 0 0' }); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Wrap long lines before paginating.
Each input line becomes one Tj operation, while pagination only checks vertical space. Normal resume paragraphs longer than the available page width will run off-page and be clipped. Add width-aware wrapping before splitting lines into pages.
Also applies to: 62-71, 88-93
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/export/pdf.ts` around lines 23 - 39, Update the line-processing flow
in the PDF export routine to wrap each rendered text line to the available page
width before pagination and PDF text operations are generated. Preserve the
existing heading, bullet, indentation, font-size, and color metadata for every
wrapped segment, and apply the same behavior to the related pagination/rendering
paths referenced by the comment.
Comment on lines
+35
to
+36
| } else if (trimmed.startsWith('- ') || trimmed.startsWith('• ')) { | ||
| allLines.push({ text: `• ${trimmed.replace(/^[-•]\s*/, '')}`, size: 11, bold: false, indent: 20, color: '0 0 0' }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Use one consistent byte encoding for PDF streams.
Line 94 calculates UTF-8 length, but Line 129 emits Latin-1 bytes. The generated • already makes /Length incorrect and is mangled under /WinAnsiEncoding; other Unicode content has the same problem. Encode text to supported WinAnsi bytes first and calculate lengths from those exact bytes, or embed a Unicode-capable font.
Also applies to: 45-46, 94-94, 110-111, 129-129
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/export/pdf.ts` around lines 35 - 36, Use one consistent
WinAnsi-compatible byte encoding throughout the PDF stream generation around
allLines: encode every emitted text value, including the bullet character and
other Unicode content, before calculating `/Length` and reuse those exact bytes
when writing the stream. Do not calculate lengths from UTF-8 strings while
emitting Latin-1 bytes; alternatively, switch the embedded font and stream
handling to a Unicode-capable encoding.
Comment on lines
+82
to
+111
| addObject('<< /Type /Catalog /Pages 2 0 R >>'); | ||
| const contentObjIds: number[] = []; | ||
|
|
||
| for (const pageLines of pages) { | ||
| let stream = ''; | ||
| let py = MARGIN_TOP; | ||
| for (const item of pageLines) { | ||
| const fontName = item.bold ? '/F2' : '/F1'; | ||
| const x = MARGIN_LEFT + item.indent; | ||
| stream += `BT\n${fontName} ${item.size} Tf\n${item.color} rg\n${x} ${py} Td\n(${escapePdfText(item.text)}) Tj\nET\n`; | ||
| py -= item.size + 4; | ||
| } | ||
| const streamObjId = addObject(`<< /Length ${Buffer.byteLength(stream)} >>\nstream\n${stream}\nendstream`); | ||
| contentObjIds.push(streamObjId); | ||
| } | ||
|
|
||
| // Pages object (object 2) references all page objects. Page objects are laid out | ||
| // immediately after this one at positions 3, 5, 7, ... (each page gets one object), | ||
| // matching the content-stream object ids produced above. | ||
| const pagesKids = pages.map((_, i) => `${3 + i * 2} 0 R`).join(' '); | ||
| addObject(`<< /Type /Pages /Kids [${pagesKids}] /Count ${pages.length} >>`); | ||
|
|
||
| for (let i = 0; i < pages.length; i++) { | ||
| addObject( | ||
| `<< /Type /Page /Parent 2 0 R /MediaBox [0 0 ${PAGE_WIDTH} ${PAGE_HEIGHT}] /Contents ${contentObjIds[i]} 0 R /Resources << /Font << /F1 5 0 R /F2 6 0 R >> >> >>`, | ||
| ); | ||
| } | ||
|
|
||
| addObject('<< /Type /Font /Subtype /Type1 /BaseFont /Courier /Encoding /WinAnsiEncoding >>'); | ||
| addObject('<< /Type /Font /Subtype /Type1 /BaseFont /Courier-Bold /Encoding /WinAnsiEncoding >>'); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
Fix the PDF object IDs; the current references produce malformed documents.
Content streams receive IDs starting at 2, but the catalog treats object 2 as /Pages. Page, parent, child, and fixed font references consequently point at the wrong object types.
Proposed object ordering
+ const pageObjectIds = pages.map((_, i) => 3 + i);
+ const contentObjectIds = pages.map((_, i) => 3 + pages.length + i);
+ const regularFontId = 3 + pages.length * 2;
+ const boldFontId = regularFontId + 1;
+
addObject('<< /Type /Catalog /Pages 2 0 R >>');
- const contentObjIds: number[] = [];
+ addObject(
+ `<< /Type /Pages /Kids [${pageObjectIds.map(id => `${id} 0 R`).join(' ')}] /Count ${pages.length} >>`,
+ );
+
+ for (let i = 0; i < pages.length; i++) {
+ addObject(
+ `<< /Type /Page /Parent 2 0 R /MediaBox [0 0 ${PAGE_WIDTH} ${PAGE_HEIGHT}] /Contents ${contentObjectIds[i]} 0 R /Resources << /Font << /F1 ${regularFontId} 0 R /F2 ${boldFontId} 0 R >> >> >>`,
+ );
+ }
for (const pageLines of pages) {
// Build stream...
- const streamObjId = addObject(/* stream */);
- contentObjIds.push(streamObjId);
+ addObject(/* stream */);
}
-
- // Remove the existing Pages and Page object creation below.Add a PDF parser/open round-trip test; header and trailer markers cannot detect broken references.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/export/pdf.ts` around lines 82 - 111, Correct the object allocation
and references in the PDF generation flow around addObject, ensuring the catalog
points to the actual Pages object and each page’s /Contents, /Parent, and font
resources reference the object IDs returned for those objects rather than
hard-coded positions. Preserve a consistent ordering for catalog, pages, content
streams, page objects, and fonts, and add a parser/open round-trip test that
verifies the generated PDF can be successfully opened.
Comment on lines
+16
to
+27
| const usage: Usage = { | ||
| interviewsThisWeek: 0, | ||
| resumeReviewsThisMonth: 0, | ||
| coverLettersThisMonth: 0, | ||
| practiceTestsThisMonth: 0, | ||
| }; | ||
|
|
||
| function guard(feature: Feature): SubscriptionCheck { | ||
| const allowed = entitlement.canAccess(feature) && entitlement.checkUsage(feature, usage).allowed; | ||
| return allowed | ||
| ? { allowed: true, remaining: null } | ||
| : { allowed: false, reason: 'Not available on the free tier' }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Forward actual usage and preserve the entitlement result.
The fixed zero-valued usage object means paid/metered services never receive each wrapper’s supplied count. guard() also replaces finite remaining values and service-provided reasons with null/a generic message. Build Usage from each wrapper’s arguments and return checkUsage()’s result after the feature-access check.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/subscription-guard.ts` around lines 16 - 27, Update guard and each
feature wrapper to construct Usage from the wrapper’s supplied count instead of
the shared zero-valued usage object. Preserve entitlement.canAccess(feature) as
the access gate, and when access is granted return the full result from
entitlement.checkUsage(feature, usage), including remaining and reason; retain
the existing denied result when feature access fails.
- Add Bun overrides for postcss@8.5.19 and uuid@14.0.1 - Remove stale package-lock.json, use bun.lock - Create 11 SVG placeholder illustrations for all image references - Create PWA manifest.json and icon SVGs - Update all .png image references to .svg in components - Remove pricing navigation and PricingPage stub (product is free) - Make SubscriptionBanner props optional - Remove pricing from ActiveView type - Verified: TypeScript build passes, Next.js build passes - Verified: Unit tests 33/33 pass, API tests 226/264 pass - Verified: bun audit clean (no vulnerabilities)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 3 of the SOLID remediation roadmap (FIX-PLAN.md): entitlement honesty (LSP). The
subscription-guard.tscheck*Access() functions looked like tier gating (they accepted tier/usage arguments) but always returnedallowed: true— a dishonest contract.use-subscription.tsreturned magic-1sentinels to mean "unlimited".What changed
EntitlementServiceinterface +FreeEntitlementimplementation (tier'free', all features allowed, limits =null= unlimited). The exportedentitlementsingleton is the single swap point: add a paid tier later by implementing the interface and swapping the instance — zero caller changes (Dependency Inversion).check*Access()helpers now delegate to theEntitlementService. Function signatures are unchanged so all existing callers (guides,questions,MockInterview,ResumeLab,CoverLetterStudio,PracticeTests) keep working without edits. The contract is now honest: gating is real (and currently open), not a silent no-op.null(unlimited) limits sourced from the service instead of-1.src/lib/pricing.ts— tier config that only powered the stubs (now truly dead).Tests / verification
tsc --noEmit: clean.eslint: clean.🤖 Generated with opencode
Summary by CodeRabbit